Implement `#:package` manipulation for file-based apps #49635

jjonescz · 2025-07-02T15:22:59Z

Part of #49200. This PR implements dotnet package add and dotnet package remove.

When more logic is shared with NuGet, more package-related commands can be implemented and some of the logic here simplified. See NuGet/Home#14390.

I've modeled the commands around their project-based counterparts. package remove is relatively simple - corresponding #:package directives are removed from the C# file, Directory.Packages.props is not considered at all. package add needs to update Directory.Packages.props if Central Package Management is enabled. It also needs to perform restore. If no version is specified by the user on the command-line, it needs to determine the latest version and put that into the package reference directive.

Copilot

Pull Request Overview

This PR extends the dotnet package commands to support file-based C# apps by introducing a --file option, updating parsing/completion, and adding source‐editor logic to manipulate #:package directives.

Introduces --file alongside --project in parsers and completion scripts
Implements add/remove of #:package directives in FileBasedAppSourceEditor and wiring in PackageAddCommand / PackageRemoveCommand
Adds extensive unit tests and updates documentation for the new file-based scenarios

Reviewed Changes

Copilot reviewed 57 out of 57 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
src/Cli/dotnet/Commands/Package/PackageCommandParser.cs	Adds `FileOption` and path‐processing logic for file-based apps
src/Cli/dotnet/Commands/Package/Add/PackageAddCommand.cs	Implements file‐based add logic, including CPM and restore support
src/Cli/dotnet/Commands/Package/Remove/PackageRemoveCommand.cs	Implements file‐based remove logic and reports removed directives
src/Cli/dotnet/Commands/Run/VirtualProjectBuildingCommand.cs	Refactors diagnostics handling and directive loading for files
src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs	New editor for inserting/removing `#:package` directives
src/Cli/dotnet/Commands/Run/RunCommand.cs	Updates error message constant for invalid option combinations

Comments suppressed due to low confidence (1)

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs:105

Typo in variable name addAfer—it should be spelled addAfter to match the intended meaning.

        CSharpDirective? addAfer = null;

src/Cli/dotnet/Commands/Run/RunCommand.cs

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs

jjonescz · 2025-07-09T08:08:57Z

@dotnet/run-file for reviews, thanks

MiYanni · 2025-07-10T21:28:08Z

src/Cli/dotnet/Commands/Package/Add/PackageAddCommand.cs

+                }
+            }
+
+            {


I know you can make arbitrary scopes like this, but what is this for?

This allows me to define the void Update(string value) local function twice - otherwise the nested one would "see" the parent one and complain it's re-declared.

I would say that this is rather confusing: consider either using different names, such as UpdateInVersion/UpdateInMetadata, or combining the local function into one that takes an extra parameter, and then wrapping them in a lambda at the return to pass either versionAttribute or metadata.

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs

jjonescz · 2025-07-14T09:06:23Z

@RikkiGibson @chsienki for reviews, thanks

src/Cli/dotnet/Commands/Package/PackageCommandParser.cs

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs

src/Cli/dotnet/Commands/Package/PackageCommandParser.cs

src/Cli/dotnet/Commands/Package/Add/PackageAddCommand.cs

src/Cli/dotnet/Commands/Package/Remove/PackageRemoveCommand.cs

test/dotnet.Tests/CommandTests/Package/Add/GivenDotnetPackageAdd.cs

documentation/general/dotnet-run-file.md

src/Cli/dotnet/Commands/Reference/Remove/ReferenceRemoveCommand.cs

jjonescz · 2025-07-18T08:26:14Z

@RikkiGibson @jaredpar for another look, thanks

jjonescz · 2025-07-21T10:58:58Z

@RikkiGibson @333fred for reviews, thanks

333fred · 2025-07-21T18:27:23Z

src/Cli/dotnet/Commands/Package/Remove/PackageRemoveCommand.cs

        }
-        if (_arguments.Count != 1)
+
+        var packageToRemove = arguments.Single();


Nit: can you just index? Or call First? No need for assertions around the length, we just checked that.

I can use pattern matching. Thanks.

333fred · 2025-07-21T18:56:54Z

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs

+                trailingNewLines = 0;
+            }
+
+            bool isEndOfLine = trivia.IsKind(SyntaxKind.EndOfLineTrivia);


Make sure you're testing with a documentation comment at the top of the file, as well as things that will generate SkippedTokensTrivia. See https://github.com/dotnet/razor/blob/main/src/Compiler/Microsoft.CodeAnalysis.Razor.Compiler/src/Language/Legacy/RoslynCSharpTokenizer.cs#L503 and surrounding cases for some examples.

333fred · 2025-07-21T20:47:53Z

test/dotnet.Tests/CommandTests/Package/Remove/GivenDotnetRemovePackage.cs

Are there tests for files with comments at the top?

Yes, see test Comments.

333fred · 2025-07-21T20:53:25Z

test/dotnet.Tests/CommandTests/Run/FileBasedAppSourceEditorTests.cs

+            """
+            /* test */
+
+            #:package [email protected]


I think this is a bit subjective, but I'd rather see:

#:package MyPackage@1.0.0 /* test */ Console.WriteLine();

RikkiGibson

Haven't gotten all the way through yet, but, will finish tomorrow and also get through the other PRs #49769 #49807 #49808.

RikkiGibson · 2025-07-22T04:17:49Z

src/Cli/dotnet/Commands/Package/Add/PackageAddCommandParser.cs

@@ -18,7 +16,7 @@ public static class PackageAddCommandParser
    .AddCompletions((context) =>
    {
        // we should take --prerelease flags into account for version completion
-        var allowPrerelease = context.ParseResult.GetValue(PrereleaseOption);
+        var allowPrerelease = context.ParseResult.GetValue(PrereleaseOption!);


It looks like the order of the declarations is the only reason the suppressions are needed. That seems unfortunate.

You're right, I can reorder, thanks.

RikkiGibson · 2025-07-22T04:38:03Z

src/Cli/dotnet/Commands/Package/Add/PackageAddCommand.cs

+                    if (!hasVersion)
+                    {
+                        skipUpdate = true;
+                        return (Revert: NoOp, Update: Unreachable, Save: Revert);


I am finding the flow here pretty wonky. It feels like some of the cases update the file on disk before we know whether that's what we should really do, and other cases wait to confirm that's what we should do before doing the update. Does it have to be that way?

I was also kinda hoping that the case where we need to update a Directory.Packages.props file could share more with the project-based version of the command. I guess that is not practical to do.

I believe that once we have NuGet/Home#14390, lots of this logic can go away. But I didn't want to block the basic experience on that.

RikkiGibson · 2025-07-22T04:44:22Z

src/Cli/dotnet/Commands/Package/PackageCommandParser.cs

+                ? (projectOrFile, AppKinds.Any)
+                : (Environment.CurrentDirectory, AppKinds.ProjectBased),
+            (true, false) => (parseResult.GetValue(FileOption)!, AppKinds.FileBased),
+            (false, true) => (parseResult.GetValue(ProjectOption)!, AppKinds.ProjectBased),


So is the "optionality" of an Option<T> indicated by giving it a nullable type argument? I am just curious if something would change for the worse here if FileOption and ProjectOption were Option<string>.

It wouldn't make a difference, the GetValue returns nullable T no matter what the original T is:

public T? GetValue<T>(Option<T> option)

RikkiGibson · 2025-07-22T04:48:13Z

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs

+        return new FileBasedAppSourceEditor
+        {
+            SourceFile = sourceFile,
+            Directives = LoadDirectives(sourceFile),


nit: if Directives is just initialized based on value of SourceFile, would it be cleaner to delete the setter for it and just rely on it being initialized on access?

RikkiGibson · 2025-07-22T04:53:15Z

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs

+
+    public void Add(CSharpDirective directive)
+    {
+        TextSpan span = DetermineWhereToAdd(directive, out var append);


Roslyn has a type TextChange, which consists of a TextSpan and string NewText, and SourceText.WithChanges() can take them to created edited versions of source files. Maybe it would be a bit cleaner to use that here?

RikkiGibson · 2025-07-22T04:54:34Z

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs

+
+        // Find the last directive of the first group of directives of the same kind.
+        // If found, we will insert the new directive after it.
+        CSharpDirective? addAfer = null;


addAfter?

RikkiGibson · 2025-07-22T04:55:47Z

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs

+        // Otherwise, we will add the directive to the top of the file.
+        int start = 0;
+
+        var tokenizer = VirtualProjectBuildingCommand.CreateTokenizer(SourceFile.Text);


IMO, this is an infrequent enough occurrence, that it would be fine to just parse, if it would make this implementation simpler. However, there's no strong need to make a change to do that.

I don't think that would be any simpler - we would get the same kind of result (syntax trivia) to work on, and parsing would just do more work than lexing does.

Implement '#:package' manipulation for file-based apps

f45fb87

jjonescz added the Area-run-file Items related to the "dotnet run <file>" effort label Jul 2, 2025

Fixup a pre-existing test

16ccaaf

jjonescz marked this pull request as ready for review July 2, 2025 17:02

jjonescz requested review from Copilot and a team July 2, 2025 17:02

Copilot AI reviewed Jul 2, 2025

View reviewed changes

src/Cli/dotnet/Commands/Run/RunCommand.cs Show resolved Hide resolved

src/Cli/dotnet/Commands/Run/FileBasedAppSourceEditor.cs Show resolved Hide resolved

DamianEdwards mentioned this pull request Jul 2, 2025

Support single file cs apphost dotnet/aspire#9612

Open

4 tasks

Merge branch 'main' into sprint-add

74be964

build-analysis bot mentioned this pull request Jul 9, 2025

Containers integration tests are flaky when the network fails to load the registry #49502

Open

MiYanni approved these changes Jul 10, 2025

View reviewed changes

jjonescz requested a review from a team July 11, 2025 09:40

jjonescz added 2 commits July 11, 2025 12:01

Merge branch 'main' into sprint-add

4640b38

Fixup bad merge

1a7b31a

jjonescz requested review from jaredpar and RikkiGibson July 15, 2025 07:41

jaredpar self-assigned this Jul 15, 2025

jaredpar reviewed Jul 15, 2025

View reviewed changes

RikkiGibson reviewed Jul 15, 2025

View reviewed changes

documentation/general/dotnet-run-file.md Show resolved Hide resolved

jjonescz added 5 commits July 16, 2025 12:24

Support more new line chars

5e4a899

Simplify directive auto reloading

5af91c4

Improve code readability

cc67ecf

Handle MSBuild bools

5f71d63

Improve tests

f548112

jjonescz mentioned this pull request Jul 16, 2025

Fix dotnet package remove command when project is not specified #49314

Closed

build-analysis bot mentioned this pull request Jul 16, 2025

The Operation will be canceled. The next steps may not contain expected logs. dotnet/dnceng#3008

Open

3 tasks

jjonescz requested review from jaredpar and RikkiGibson July 16, 2025 14:19

build-analysis bot mentioned this pull request Jul 16, 2025

dotnet-watch tests failing #40006

Open

marcpopMSFT reviewed Jul 16, 2025

View reviewed changes

src/Cli/dotnet/Commands/Reference/Remove/ReferenceRemoveCommand.cs Show resolved Hide resolved

Add tests from dotnet#49314

d6d0778

This was referenced Jul 17, 2025

The active test run was aborted. Reason: Test host process crashed dotnet/dnceng#451

Open

Template engine tests hitting known parallelism issue in GetVisualStudioInstances #44878

Open

333fred reviewed Jul 21, 2025

View reviewed changes

RikkiGibson reviewed Jul 22, 2025

View reviewed changes

+                              }
+                          }
+                          {

Implement #:package manipulation for file-based apps #49635

Are you sure you want to change the base?

Implement #:package manipulation for file-based apps #49635

Uh oh!

Conversation

jjonescz commented Jul 2, 2025 • edited by DamianEdwards Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

jjonescz commented Jul 9, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

jjonescz commented Jul 14, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jjonescz commented Jul 18, 2025

Uh oh!

jjonescz commented Jul 21, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

RikkiGibson left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Implement `#:package` manipulation for file-based apps #49635

Implement `#:package` manipulation for file-based apps #49635

jjonescz commented Jul 2, 2025 •

edited by DamianEdwards

Loading